Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update to MOM6 main 20230504 updating #113

Merged
merged 239 commits into from
May 22, 2023

Conversation

jiandewang
Copy link
Collaborator

@jiandewang jiandewang commented May 5, 2023

MOM6 main repository is updated on 20230504, need to make corresponding updating in dev/emc. See detail at mom-ocean#1599 and mom-ocean#1594. Related dev/emc issue is #112. No answer change is expected in UFS.

gustavo-marques and others added 30 commits July 26, 2022 09:58
Address comment from reviewer by adding units to covTS and varS.
* Add ``implicit none ; private`` to this module;
* Put module variables into the control structure for this module;
* Add the description of the units for all real variables;
* Add a consistent two-point indent throughout the module .

TODO:
Without further modifications, adding ``private`` to the control
structure of this module will break the model. Currently, MOM.F90
needs access to ``use_stoch_eos``, ``stanley_coeff``, and some of
the diagnostic ids.
…the ideal age

module. This PR also adds the ability to use the actual BL depth that is diagnosed by the
active BL scheme inside the ideal age module (for all ideal age tracers).
Bring in latest main changes (GFDL to main 2022-07-21)
…candidate-main-2022-08-10

Merge GFDL to main (2022-08-10)
In preparation for implementing the option to apply a vertical
smooth filter in the Richardson number multiple times, the
parameter SMOOTH_RI (logical) was renamed to N_SMOOTH_RI (interger).
If N_SMOOTH_RI = 0 (default), smoothing is not performed. If
N_SMOOTH_RI > 0, smoothing will be applied N_SMOOTH_RI times.
Currently, there are two diagnostics related to the gradient Richarson
number and these are described as follows:

* ri_grad_shear         : Gradient Richarson number used by
  MOM_CVMix_shear module;

* ri_grad_shear_smooth  : Smoothed gradient Richarson number used by
  MOM_CVMix_shear module.

The description for ri_grad_shear is misleading. If smoothing is applied,
ri_grad_shear *is not* the RI number used by MOM_CVMix_shear module. In
this commit. I propose to avoid this potential confusion by renaming
ri_grad_shear_smooth to ri_grad_shear_orig and, if N_SMOOTH_RI > 0,
use ri_grad_shear to store the smoothed profiles.

* ri_grad_shear_orig  : Original gradient Richarson number, before smoothing
  was applied. This is part of the MOM_CVMix_shear module and only available
  when N_SMOOTH_RI > 0.

No change in answers for GMOM.
This commit adds the option to smooth the gradient Richardson
number multiple times using a 1-2-1 filter. The number of times
that the filter is applied is controlled by parameter N_SMOOTH_RI.
The correct order is lon_min lon_max lat_min lat_max ...
and not lat_min lat_max lon_min lon_max.
  Created the new module remapping_attic to hold older versions of remapping
code that are no longer used by MOM6.  The subroutines is PosSumErrSignificant,
remapByProjection, remapByDeltaZ and integrateReconOnInterval were moved to
remapping_attic, where they can be tested by calling remapping_attic_unit_tests.
The hard-coded old_algorithm logical module variable and the code it wraps were
also eliminated.  Also added a schematic description of the units of the real
variables in the various routines in MOM_remapping and corrected some spelling
errors.  All answers are bitwise identical.
  Moved interpolate_column and reintegrate_column (without changing anything)
from MOM_diag_vkernels.F90 to MOM_remapping.F90 and incorporated the tests
that had been in diag_vkernels_unit_tests into remapping_unit_tests.  The
entire MOM_diag_vkernels.F90 file was then removed.  All answers are bitwise
identical, although the module for two public routines was changed and a third
was eliminated.
  Remove missing_value arguments to interpolate_column and reintegrate_column,
instead using 0 for the values in vanished cells.  This change helps to address
github.com/mom-ocean/issues/769.  Also added comments schematically
describing some of the argument units.  Because 0 was already being used for the
missing value (except in unit tests), all solutions are bitwise identical.
  Added the new subroutine check_remapped_values with the duplicative error
checking code in remapping_core_h and remapping_core_w, both to reduce code
volume and promote code coverage, and to make the substance of these two
routines easier to follow.  All answers are bitwise identical.
- Adds `.gitlab/pipeline-ci-tool.sh` to enact most of the stages of the gitlab CI pipeline
  - enables interactive/command-line reproduction of the pipeline
  - `.gitlab/pipeline-ci-tool.sh` is documented in .gitlab/README.md with instructions
    on how to use at the command line and what each function is doing
- All commands formerly in .gitlab-ci.yml are now one-line invocations of `.gitlab/pipeline-ci-tool.sh`
  so .gitlab-ci.yml is now considerably smaller and easier to read with statements like
  `.gitlab/pipeline-ci-tool.sh mrs-compile debug_gnu` or `.gitlab/pipeline-ci-tool.sh check-params gnu`
- Previously, all results were compared again the stored regression answers. This meant that
  any error (e.g. layout) would show up as a fail of all types. We use the regression answers to
  check the repro-symmetric mode and then compare everything else to repro-symmetric or other results
  as appropriate. This allows us to distinguish between types of errors. The GH actions are doing it
  this way, and we originally did this in the first forms of the pipeline, but in the last re-factor
  I lazily switched to using the regression answers for everything.
- The subroutine categorize_axes cannot find the axes in ice restart files and gives warnings
WARNING from PE     0: categorize_axes: Failed to identify x- and y- axes in the axis list (xaxis_1, yaxis_1, Time) of a variable being read from INPUT/ice_model.res.nc
- This leads to an incorrect initializations and a subsequent sat.vap.press.overflow crash when using
infra/FMS2
- Same experiment runs fine with infra/FMS1
- After the fix the infra/FMS1 and infra/FMS2 answers are bitwise
identical
- The subroutine categorize_axes cannot find the axes in ice restart files and gives warnings
    WARNING from PE     0: categorize_axes: Failed to identify x- and y- axes in the axis list (xaxis_1, yaxis_1, Time) of a variable being read from I
NPUT/ice_model.res.nc
- This leads to an incorrect initializations and a subsequent sat.vap.press.overflow crash when using
    infra/FMS2
- Same experiment runs fine with infra/FMS1
- After the fix the infra/FMS1 and infra/FMS2 answers are bitwise
    identical
Added a line initializing the string Cartesian to a blank string in categorize_axes, so that it not be uninitialized when it is used a few lines later.
  Set the interpolation weights inside of interpolate_column to explicitly be
the complement of one another, thereby saving an extra division at each point
and reducing the number of variables that need to be stored, in preparation for
the creation of a separate subroutine to find interface positions.  This commit
is mathematically equivalent to what was there before, and the extensive unit
testing of interpolate_column is still passing, but it changes the value of some
interpolated interface diagnostics at the level of roundoff (but not the MOM6
solutions themselves, as they do not depend on interpolate_column yet).
This patch introduces a new autoconf macro, AX_FC_CHECK_C_LIB, which
confirms if the Fortran compiler can be linked to the netCDF C library.
As with other netCDF tests, the nc-config tool is used if necessary (and
available).

This resolves some recent issues on platforms where netCDF and
netCDF-Fortran are installed in separate locations, with different
library directories (-L).

It also resolves some false assumptions in configure.ac which presumed
equivalent access by the configured C and Fortran compilers.
Previously, we would test if the C compiler could be linked to netCDF,
and then assume that the Fortran compiler shared the same relationship.
We now use the Fortran compiler for both C and Fortran tests.

This patch fixes many issues observed on MacOS systems, including some
persistent problems on the GitHub Actions MacOS tests.  For example, we
can now use the default GCC 12 compilers, rather than forcing a rollback
to GCC 11.
This patch fixes some issues with testing of C bindings in Fortran.
Specifically, some tests are using a C compiler which may be
unconfigured, causing unexpected errors.

The autoconf script now uses the Fortran compiler to test these
bindings, rather than using the C compiler to test for their existence.
A new macro (AX_FC_CHECK_BIND_C) was added to run these tests.

This achieves the actual goal (test of Fortran binding) on top of the
original goal (availability of C function), while ensuring that the actual
compiler of interest (FC) is used in the test.

Two C-based tests are still present in the script for testing the size
of jmp_buf and sigjmp_buf.  The C compiler is now configured with the
AX_MPI macro, and is only used to determine the size of these structs.
* Setup OBC segments for COBALT/OBGC tracers

    - These are updates required to setup OBC segments for OBGC tracers.
    - Since COBALT package has more than 50 tracers using the MOM6 table
      mechanism for setting up OBC segments is not feasible. Rather, this
      update delegates such setup to mechanims used in ocean_BGS tracers
      leaving MOM6 mechanism for native tracers intact.
    - Fixed issues caught by MOM6 githubCI

* Add capability to change obc segment update period

- COBALT tracers do not need as frequent segment bc updates and can
  use a larger update period to speed up the model.
  This commit introduces a new parameter DT_OBC_SEG_UPDATE_OBGC
  that can be adjusted for obc segment update period.
- This commit applies the change only to BGC tracers but can easily
  be changed to apply for all.

* Insert missing US%T_to_sec

- The unit conversion factor was missing causing a crash in a newer test.

* Updates from Andrew Ross

- Avoid low initial values in the tracer reservoirs

* Per Andrew Ross review

* corrected indentation per review

* Avoid using module vars per review request

- Reviewer asked to avoid using module variables with "save" attributes.
- This commit hides the module variables inside the existing OBC type.

* Coding style corrections per review

* Modification per review: do_not_log if .not.associated(CS%OBC)

Co-authored-by: Robert Hallberg <[email protected]>
In this PR an option is added to use ice viscosity computed from the observed surface velocity, computed by the model and use a constant value (for debugging purposes). A new (char) parameter "ICE_VISCOSITY_COMPUTE" is introduced; its values can be "MODEL" (the ice viscosity computed by the model); "OBS" the ice viscosity is computed at the preprocessing step and read from a file (its name is defined by the parameter "ICE_STIFFNESS_FILE") into a variable with a name defined by "A_GLEN_VARNAME" parameter; "CONSANT" is a constant value defined by a parameter "A_GLEN". These changes are in MOM_ice_shelf_dynamics.F90. Minor changes are done to MOM_ice_shelf_initialize.F90 to correct units, scales.
marshallward and others added 19 commits February 28, 2023 13:07
Erratic slowdowns in our Lustre filesystem mean that test runtimes are
largely unpredictable.  This patch increases the runtimes from 20min to
1hr, to better cope with this issue.
  Corrected the units in the get_param call for WAVE_HEIGHT_SCALE_FACTOR, and
corrected the units descriptions in comments of 22 wind stress related variables
in 6 driver routines, from [R L Z T-1 ~> Pa] to [R L Z T-2 ~> Pa], but the
actual conversion factors in the code are correct.  Also fixed 42 other
inconsistent units in comments in 28 files scattered throughout the MOM6 code.
WAVE_HEIGHT_SCALE_FACTOR was added in December 2022 as a part of PR mom-ocean#289 to
dev/gfdl. These inconsistent units were detected because they do not match the
patterns of other valid units; most are recent additions.  Apart from a single
unit in a get_param call, only comments are changed, and all answers are bitwise
identical.
  Restored an else that was inadvertently deleted as a part of code clean up in
MOM-ocean/MOM6 PR mom-ocean#1127 on June 5, 2020.  This bug causes bulkmixedlayer to be
called twice (with cumulative effects) when 0. < ML_MIX_FIRST < 1., and not to
be called at all when ML_MIX_FIRST = 1.  This bug only applies to cases where
the bulk mixed layer is enabled by setting BULKMIXEDLAYER=True and
USE_REGRIDDING=False (i.e., in layered mode configurations with active
thermodynamics), however because the default value of ML_MIX_FIRST = 0, this bug
does not appear to be used in any active test cases, and it went undetected when
it was introduced.  All answers in the MOM6-examples test suite are bitwise
identical, but this could change answers in some cases.
  Revised write_energy so that only the root_PE attempts to open, reopen or
write to a netcdf file.  Although FMS can handle cases where multiple PEs make
the same calls with internal logic, this change avoids requiring such internal
(hidden) logical tests, and instead is more explicit on what is actually
intended.  This change is complementary to MOM6 dev/gfdl PR mom-ocean#328, which adds
internal logic to handle the case where all PEs are making a call to reopen a
single netcdf file.  All answers and output are bitwise identical.
Fixed two horizontal indexing bugs in `get_field_nc`, where the difference
between the array starting index (always 1 in this subroutine) and the values
in the handle argument were not being taken into account when the array was
being passed in with only its computational domain.

Also initialized the internal `unlim_index` array in `get_netcdf_fields` to fix
a problem with using an uninitialized array that was being flagged when run in
debug mode.

With this commit, the model is once again reproducing the expected answers when
rescaling is applied for vertical distances or time.

Co-authored-by: Marshall Ward <[email protected]>
Add "PPM_CW" as an option for INTERPOLATION_SCHEME and REMAPPING_SCHEME.
This implements the original Colella and Woodward (1984) edge calculation
for PPM. It computes 4th order explicit edge values but constrains
them to produce a monotonic profile, which is particularly effective
for remapping.

INTERPOLATION_SCHEME="PPM_CW" is identical to "REMAPPING_PPM_HYBGEN",
but hybgen_PPM_coefs has been replaced by edge_values_explicit_h4cw
and PPM_monotonicity for flexibility and to simplify upgrades.
Answers with existing INTERPOLATION_SCHEME options are unchanged.

REMAPPING_SCHEME="PPM_CW" is a new option which can perform better
than "P1M_H2" when used with INTERPOLATION_SCHEME="PPM_CW".  Answers
with existing REMAPPING_SCHEME options are unchanged.

HYCOM1 regridding walks a monotonic density profile to locate the
new interface locations where the interface density equals the target
density.  However, it assumes that moving one interface has no effect
on the density at all other interfaces and this need not be the case.
When regridding, with HYCOM1_ONLY_IMPROVES=True, an interface is only
moved if this improves the fit to its target density.  The default of
False does not change answers.
  Updated check_MOM6_scaling_factors and compose_dimension_list to reflect that
fact that MOM6 is now doing dimensional consistency testing for temperature (via
[C ~> degC]) and salinity (via [S ~> ppt]), with an expanded dimension of the
scaling key from 6 to 8 and additional calls to add_scaling.  Also updated the
weights on the add_scaling calls, which are essentially counts of the frequency
of the various unit descriptors in the MOM6 code, to reflect only the counts of
variables with doxygen comments (i.e., arguments, function return values and
elements of types) but excluding the user, framework and diagnostics directories
and the passive tracer packages.  All model solutions are bitwise identical, but
there will be updated suggestions for combinations of scaling factors that
minimize the aliasing of the units that are used.
…023-03-02

NCAR to main candidate branch (2023-3-02)
  Removed the coord_slight module and all calls to it, and obsoleted all
run-time parameters that are exclusively related to it.  This code was an
attempt from 2015 to define an appropriate hybrid vertical coordinate for global
climate modeling, but it never worked very well (usually falling apart in the
second year), and it has not been used in any publication or active model for
many years.  The test case that exercised this coordinate in the MOM6-examples
test suite is also being removed via MOM6-examples PR mom-ocean#388.  The coord_SLight
code is being eliminated altogether now to simplify the MOM6 code base and
reduce the volume of untested and unused code.  All answers in all known MOM6
configurations in active use are bitwise identical, although there is a remote
chance that someone somewhere might be using the SLIGHT coordinate.
  Fixed a bug in MOM_calculate_grad_Coriolis() that was causing the model to
hang due to mismatched halo updates when GLOBAL_INDEXING = True.  Also added
missing callTree (a.k.a. granny tracker) calls at the start and end of the same
routine.  All answers are bitwise identical in any cases that worked before.
Resolving conflict in MOM_CVMix_shear.F90

Conflict due to introduction of changes to Ri smoothing in CVMix in
main, alongside changes to units in comments in dev/gfdl.
Due to some machines reporting a regression in the mixed layer
restratification code, this patch reverts the calculation of the growth
time in a separate function.

Most of the content related to comments and parameter setup have been
retained, even if those parameters are no longer used.
…ate-2023-04-06

GFDL to main (2023-04-06)
@codecov-commenter
Copy link

codecov-commenter commented May 5, 2023

Codecov Report

Merging #113 (977e6c3) into dev/emc (55e0472) will increase coverage by 3.09%.
The diff coverage is 42.35%.

❗ Current head 977e6c3 differs from pull request most recent head 400bd21. Consider uploading reports for the commit 400bd21 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff             @@
##           dev/emc     #113      +/-   ##
===========================================
+ Coverage    33.99%   37.08%   +3.09%     
===========================================
  Files          259      264       +5     
  Lines        70267    74361    +4094     
  Branches     13020    13784     +764     
===========================================
+ Hits         23885    27577    +3692     
+ Misses       41880    41690     -190     
- Partials      4502     5094     +592     
Impacted Files Coverage Δ
...g_src/drivers/solo_driver/MESO_surface_forcing.F90 0.00% <0.00%> (ø)
...g_src/drivers/solo_driver/user_surface_forcing.F90 0.00% <ø> (ø)
...c/external/GFDL_ocean_BGC/generic_tracer_utils.F90 0.00% <0.00%> (ø)
...src/external/database_comms/MOM_database_comms.F90 0.00% <0.00%> (ø)
...ernal/database_comms/database_client_interface.F90 0.00% <0.00%> (ø)
src/ALE/MOM_hybgen_regrid.F90 0.00% <0.00%> (ø)
src/ALE/MOM_hybgen_remap.F90 0.00% <0.00%> (ø)
src/ALE/P1M_functions.F90 0.00% <0.00%> (ø)
src/ALE/P3M_functions.F90 0.00% <0.00%> (ø)
src/ALE/PCM_functions.F90 100.00% <ø> (+20.00%) ⬆️
... and 172 more

... and 26 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@DeniseWorthen DeniseWorthen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving based on pre-testing showing no-baseline change.

@FernandoAndrade-NOAA
Copy link

@JessicaMeixner-NOAA, Testing is complete on PR #1736, could you please merge this sub-pr, as it's been approved?

@jiandewang
Copy link
Collaborator Author

@JessicaMeixner-NOAA, Testing is complete on PR #1736, could you please merge this sub-pr, as it's been approved?

@FernandoAndrade-NOAA I am going to merge MOM6 and do the git revert

@jiandewang jiandewang merged commit fdbfa25 into NOAA-EMC:dev/emc May 22, 2023
@jiandewang
Copy link
Collaborator Author

MOM6 merged, ufs-weather-model submodule reverted

@jiandewang jiandewang deleted the feature/update-to-main-20230504 branch March 19, 2024 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.